-
Notifications
You must be signed in to change notification settings - Fork 4
fix: eidw change auth flow text #296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughRenamed the button label in the logged-in drawer within the scan-qr page from “Stay in App” to “Ok”. No logic, control flow, or exported/public signatures were changed. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte (2)
611-621
: Optional: Consider a more action-oriented label for clarity and consistencySince the callback closes the drawer, navigates to /main, and resumes scanning, a label like “Done”, “Back to Wallet”, or “Continue” could reduce ambiguity. Also verify your design system’s capitalization (“OK” vs “Ok”) and localization strategy to ensure consistency.
Example tweak:
- Ok + Done
614-618
: Avoid re-starting the scanner right before navigationCalling startScan() immediately after goto("/main") may briefly reinitialize camera scanning even though we’re leaving the page, which is unnecessary and could cause a visual flash. The onDestroy cleanup will cancel anyway. Consider removing startScan() here.
Proposed change:
callback={() => { loggedInDrawerOpen = false; goto("/main"); - startScan(); }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte
(1 hunks)
🔇 Additional comments (2)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte (2)
620-621
: Copy change LGTM — clearer and less misleading“Ok” aligns with the PR intent to avoid the misleading implication of “Stay in App.” No behavioral changes introduced.
620-621
: No remaining “Stay in App” instances foundRipgrep across the entire repository returned zero matches for “stay in app” (case-insensitive) and its common variants, confirming all occurrences have been updated.
Description of change
changes the technically misleading text "stay in app" to "ok"
Issue Number
n/a (294, dont close, has multiple)
Type of change
How the change has been tested
manual
Change checklist
Summary by CodeRabbit